use urls from zos-config if exists#33
Conversation
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
from
6831098 to
2bade3b
Compare
Signed-off-by: Ashraf Fouda <ashraf.m.fouda@gmail.com>
Signed-off-by: Ashraf Fouda <ashraf.m.fouda@gmail.com>
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
2 times, most recently
from
2d6d641 to
b41608f
Compare
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
2 times, most recently
from
5655d0d to
13533ba
Compare
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
from
13533ba to
87e059f
Compare
Eslam-Nawara
requested review from
Omarabdul3ziz,
ashraffouda,
muhamadazmy and
rawdaGastan
as code owners
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
from
2683a49 to
67517d3
Compare
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
from
d476ac6 to
4c22507
Compare
Eslam-Nawara
force-pushed
the
load-config-urls-from-zos-config
branch
from
eee4faa to
69773c0
Compare
| V4HubURL []string `json:"v4_hub_url"` | ||
|
|
||
| // we should not be supporting flist url or hub storage from zos-config until we can update them on runtime | ||
| // FlistURL string `json:"flist_url"` |
There was a problem hiding this comment.
let's have this flisturl configurable just in case we lost our urls again we can boot new machines
There was a problem hiding this comment.
we need to verify that this will actually allow us to deploy a new machine and will not break the node entirely, will start working on that after verifying all changes done are working properly and well tested in all zos types, then will do that
There was a problem hiding this comment.
no, if our domains r down, we will not be able to start new machines and this is the reason of having this required to be changed so plz do ut
| env.GeoipURLs = geoip | ||
| } | ||
|
|
||
| // flist url and hub storage urls shouldn't listen to changes in config as long as we can't change it at run time. |
There was a problem hiding this comment.
yes, but if we faced samething when all *.tf were blocked then we will not be able to boot new machines and all those configurable urls will not help unless we can connect to the new storage url
| } | ||
| return hubStorage | ||
| env := environment.MustGet() | ||
| return env.HubStorage |
There was a problem hiding this comment.
this should return the hubstorage based on if this is v3 or v4 here u r returning one hubstorage all the time
hubStorage = "zdb://hub.threefold.me:9900"
hubv4Storage = "zdb://hub.threefold.me:9940
they have diff port
even if u r doing that while creating the env but since u have this getter method make sure u use it every where and do ur logic here otherwise remove this func if not used
There was a problem hiding this comment.
environment.Must get returns hubstorage with v3 if the node is of type v3 and v4 if the node is of type v4, so we don't need to do the check again here
There was a problem hiding this comment.
otherwise remove this func if not used
^^
| err = exec.CommandContext(ctx, "ping", "-c", "3", "-q", ip).Run() | ||
| if err != nil { | ||
| log.Err(err).Msgf("failed to ping node %d", node.NodeID) | ||
| log.Debug().Err(err).Msgf("failed to ping node %d", node.NodeID) |
There was a problem hiding this comment.
why did u change this to debug? this should be visible in the node logs that doesn't booted with debug flag
There was a problem hiding this comment.
it didn't have any log level. This error doesn't break the flow, the node logs it and tries to ping another node, so I thought it should be of level debug
There was a problem hiding this comment.
t didn't have any log level.
so let's make it warning?
3 participants
Description
use config managed by environment.go from zos-config and use the ones defined in environment.go as default values
Related Issues
Checklist